Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace deprecated kustomize config patchesStrategicMerge #131

Merged
merged 3 commits into from
Jun 13, 2023
Merged

Replace deprecated kustomize config patchesStrategicMerge #131

merged 3 commits into from
Jun 13, 2023

Conversation

AndiDog
Copy link
Contributor

@AndiDog AndiDog commented May 3, 2023 •
edited
Loading

Indirect part of giantswarm/roadmap#2217 since I have to develop CAPI changes and test them.

This PR switches to using our own fork and images of cluster-api (see new main branch in https://github.com/giantswarm/cluster-api). It's consistent to how we manage our CAPA fork – from which I also took parts of the newer "generate patches" script. Once merged, we can turn off mirroring upstream images (retagger). (will move the fork part into another PR)

Since kustomize does not support multiple patches in one file anymore (kubernetes-sigs/kustomize#5049), I split those into several files. Hence the large diff.

Checklist

  • Update changelog in CHANGELOG.md.

@AndiDog AndiDog requested review from a team May 3, 2023 09:33
@AndiDog AndiDog requested a review from a team as a code owner May 3, 2023 09:33
@AndiDog AndiDog marked this pull request as draft May 24, 2023 08:59
@AndiDog AndiDog marked this pull request as ready for review June 5, 2023 09:59
@nprokopic
Copy link
Contributor

Since kustomize does not support multiple patches in one file anymore (kubernetes-sigs/kustomize#5049), I split those into several files. Hence the large diff.

Would it be possible to have this and the switch to fork in separate PRs? Both are non-trivial and important changes (one to structure of the repo, another to what is the source of truth for generating manifests here), so IMO we should not mix those in a single PR, if we can have it done in two steps.

In addition to switching to fork, and separating kustomize patches to separate files, is there some other source of / reason for some of the changes?

@AndiDog AndiDog changed the title Switch to using Giant Swarm fork for built images, replace deprecated kustomize config patchesStrategicMerge Replace deprecated kustomize config patchesStrategicMerge Jun 6, 2023
@AndiDog
Copy link
Contributor Author

AndiDog commented Jun 6, 2023

In addition to switching to fork, and separating kustomize patches to separate files, is there some other source of / reason for some of the changes?

Apart from pointing to the fork, which I have now removed from this PR, all changes are about getting the kustomize build to work with the latest version (e.g. splitting patches).

@AndiDog
Copy link
Contributor Author

AndiDog commented Jun 12, 2023

@nprokopic any more concerns?

@nprokopic
Copy link
Contributor

nprokopic commented Jun 13, 2023 •
edited
Loading

any more concerns?

Nope.

Thanks a lot for working on this! 🥳 🙇

@AndiDog AndiDog merged commit cab54df into master Jun 13, 2023
@AndiDog AndiDog deleted the kustomize-fixes branch June 13, 2023 13:09
@AndiDog AndiDog mentioned this pull request Jun 26, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nprokopic nprokopic nprokopic approved these changes

@fiunchinho fiunchinho fiunchinho approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AndiDog @nprokopic @fiunchinho